-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(map.jinja): load a configurable list of YAML files #236
feat(map.jinja): load a configurable list of YAML files #236
Conversation
This is a first draft to see if it pass CI and to show to the community that it can be used quite easily. I'll rebase this PR with properly split commits when everybody will agree. The
--- template-formula/TEMPLATE/map.jinja 2020-02-15 13:37:13.000000000 +0100
+++ mysql-formula/mysql/map.jinja 2020-02-15 15:33:39.000000000 +0100
@@ -1,6 +1,13 @@
# -*- coding: utf-8 -*-
# vim: ft=jinja
+{# mysql-formula specific initialisation #}
+{%- set py_ver_settings = {
+ 2: {'pythonpkg': 'python-mysqldb'},
+ 3: {'pythonpkg': 'python3-mysqldb'},
+ } %}
+
+
{#- Get the `tplroot` from `tpldir` #}
{%- set tplroot = tpldir.split('/')[0] %}
@@ -12,6 +19,10 @@
{%- do salt.log.debug('map.jinja: initialise parameters from ' ~ _defaults_filename ) %}
{%- import_yaml _defaults_filename as default_settings %}
+{# Reproduce previous behaviour #}
+{%- set default_settings = salt.slsutil.merge(py_ver_settings[grains.pythonversion[0]],
+ default_settings) %}
+
{# List of sources to lookup for parameters #}
{%- do salt.log.debug('map.jinja: lookup map.jinja configuration sources') %}
{# Fallback to previously used grains #}
@@ -66,12 +77,13 @@
{# Merge the config dict #}
{%- set config = salt.slsutil.merge(config, _config['formula']) %}
-{#- Change **TEMPLATE** to match with your formula's name and then remove this line #}
-{%- do salt.log.debug('map.jinja: save parameters in variable "TEMPLATE"') %}
-{%- set TEMPLATE = config %}
+{%- do salt.log.debug('map.jinja: save parameters in variable "mysql"') %}
+{%- set mysql = config %}
{#- Post-processing for specific non-YAML customisations #}
{%- if grains.os == 'MacOS' %}
-{%- set macos_group = salt['cmd.run']("stat -f '%Sg' /dev/console") %}
-{%- do TEMPLATE.update({'rootgroup': macos_group}) %}
+{%- set macos_user = salt['pillar.get']('mysql:user', salt['cmd.run']("stat -f '%Su' /dev/console")) %}
+{%- set macos_group = salt['pillar.get']('mysql:group', salt['cmd.run']("stat -f '%Sg' /dev/console")) %}
+{%- do mysql.macos.update({'user': macos_user}) %}
+{%- do mysql.macos.update({'group': macos_group}) %}
{%- endif %} Maybe we could have a uniq Regards. |
d5a1f78
to
5219dce
Compare
@baby-gnu Thanks for setting this up. I've run a quick initial test locally and the map appears to be identical. When I get a chance, I intend to test this across all of the platforms, to compare the map output before and after. |
Thanks @myii. I reproduce the failures on |
5677401
to
53e94f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this looks so much better, finding out which parameter is set as default.
@baby-gnu So I've set up two jobs in Travis to show us the
I'm running one instance for each platform and the same instances in both, so that we can get a comparison. So near the end of each Travis log you'll find the map output in YAML (the so called
Results in the following diff: --- yd01
+++ yd02
@@ -19,35 +19,13 @@
client:
port: 33306
socket: /var/lib/mysql-socket/mysql.sock
- isamchk:
- key_buffer_size: 16M
mysqld:
- basedir: /usr
- bind_address: 127.0.0.1
datadir: ~/mysql/datadir
- expire_logs_days: 10
- key_buffer_size: 16M
- lc_messages_dir: /usr/share/mysql
- max_allowed_packet: 16M
- max_binlog_size: 100M
- pid_file: /var/run/mysqld/mysqld.pid
port: 33306
- query_cache_limit: 1M
- query_cache_size: 16M
- skip_external_locking: noarg_present
socket: /var/run/mysqld/mysqld.sock
- thread_cache_size: 8
- thread_stack: 192K
- tmpdir: /tmp
user: myself
mysqld_safe:
- nice: 0
plugin-dir: ~/mysql/plugins
- socket: /var/run/mysqld/mysqld.sock
- mysqldump:
- max_allowed_packet: 16M
- quick: noarg_present
- quote_names: noarg_present
database:
- foo
- name: bar So the idea is to compare each platform to ensure that the map remains identical for each and every platform. At the moment, it will have to be a manual diff. Previously, I've been able to do something a little more sophisticated locally but I'm short on time at the moment, so this will have to do. If you can look at the above and any other diff issues, that would be really useful. I'll try to help with the comparisons later on, when I get some time. |
This is because I made a huge mistake: https://travis-ci.org/myii/mysql-formula/jobs/651071506#L1217 |
@baby-gnu No problem, let me re-run |
@baby-gnu Excellent, that's doing better. |
@baby-gnu This is a bug reported upstream (for
We could attempt to run the patch here to see if it works for us but that will have to be in a different PR. |
I was wondering if it was due to my PR and it's not, I think we have enough work to do ;-) |
@baby-gnu @aboe76 Perfect! All 14 platforms are identical, so we're well on the way. So the remaining stages:
Please let me know if I missed something. |
Lookup the configuration for `map.jinja` from: 1. builtin default `['osarch', 'os_family', 'os', 'osfinger', 'id']` 2. `defaults.yaml`: optional define a formula specific `map.jinja:sources` 3. global configuration lookup `map.jinja:sources` 4. formula specific `<tplroot>:map.jinja:sources` to find which configuration YAML files to load. The hard coded default is backward compatible and add the minion id at the end of the list. If an entry does not match a `salt['config.get']` parameter, it's used as a literal file path (with `.yaml` added if it's missing). Each YAML file are formatted with the following top level keys: - `values`: contains the the parameter values - `strategy` (optional): define the merge strategy for the function `salt.slsutils.merge` (aggregate, list, overwrite, recurse, smart). The default is `smart`. - `merge_lists` (optional): boolean to merge lists or overwrite them The parameters values are merged in the following order: 1. initialize default values from `defaults.yaml` 2. merge the values from the YAML file `<tplroot>/parameters/<config>/<configvalue>.yaml` in the order defined by `map.jinja:sources` 3. merge the values from `<tplroot>:lookup` 4. merge the values from `salt['config.get'](<tplroot>)`
* mysql/parameters/defaults.yaml: remove the top level key.
590184c
to
2458f1c
Compare
I'll do a newt PR based on v5 |
PR progress checklist (to be filled in by reviewers)
What type of PR is this?
Primary type
[build]
Changes related to the build system[chore]
Changes to the build process or auxiliary tools and libraries such as documentation generation[ci]
Changes to the continuous integration configuration[feat]
A new feature[fix]
A bug fix[perf]
A code change that improves performance[refactor]
A code change that neither fixes a bug nor adds a feature[revert]
A change used to revert a previous commit[style]
Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)Secondary type
[docs]
Documentation changes[test]
Adding missing or correcting existing testsDoes this PR introduce a
BREAKING CHANGE
?No.
Related issues and/or pull requests
Describe the changes you're proposing
I propose to modify
map.jinja
to load a configurable list of YAML files.The configuration of
map.jinja
is lookup in the following order:defaults.yaml
for the maintainer (can be overrode by the user)map.jinja:sources
to override globally the configuration for all formulasmysql:map.jinja:sources
to override the configuration only for this formulaI split all the
os*map.yaml
underparameters/<config>/<configvalue>.yaml
.osarch
,os_family
,os
,osfinger
, … is split in a separate file instead of a single YAML file per configurationDebian
os_family
without the need to maintain the othersos_family
(oros
, …) by providing a single file in itsfile_roots
map.jinja:sources
(or the formula specificmysql:map.jinja:sources
), for example:['osarch', 'os_family', 'os', 'osfinger', 'domain', 'roles', 'id']
domain/example.net.yaml
,domain/example.org.yaml
,roles/web.yaml
,roles/ssh.yaml
,id/minion1.example.org.yaml
gitfs
Pillar / config required to test the proposed changes
Debug log showing how the proposed changes work
The first
sls
usingmapj.jinja
ismysql.databases
, here is the debug log during the import ofmap.jinja
The following import of
map.jinja
is less verbose, for example, this is themysql.user
debug log comming just after themysql.database
:Documentation checklist
README
(e.g.Available states
).pillar.example
.Testing checklist
state_top
).Additional context